Skip to content

✨ feat: defer interaction and follow on permission dialogs#2328

Merged
mkmccarty merged 1 commit intomainfrom
mm-branch-1
Apr 5, 2026
Merged

✨ feat: defer interaction and follow on permission dialogs#2328
mkmccarty merged 1 commit intomainfrom
mm-branch-1

Conversation

@mkmccarty
Copy link
Copy Markdown
Owner

No description provided.

Copilot AI review requested due to automatic review settings April 5, 2026 18:42
@mkmccarty mkmccarty merged commit cdaf020 into main Apr 5, 2026
8 checks passed
@mkmccarty mkmccarty deleted the mm-branch-1 branch April 5, 2026 18:42
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the boost permission dialogs’ button handlers to acknowledge component interactions using a deferred update response, then performs a subsequent edit to close the dialog and display the result text.

Changes:

  • Switched permission button acknowledgements from InteractionResponseUpdateMessage to InteractionResponseDeferredMessageUpdate.
  • Added a second step to update the dialog message (clear components + set final content) after deferring.
  • Updated the “close” button text response to “I understand”.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
src/boost/leaderboard_permission.go Defers button interaction acknowledgement and then edits the permission dialog message; changes close-message text.
src/boost/coop_status_permission.go Same deferred-ack + follow-up edit pattern applied to coop-status permission dialog; changes close-message text.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Content: &content,
Components: &emptyComponents,
}
_, err = s.FollowupMessageEdit(i.Interaction, i.Message.ID, &edit)
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ShowLeaderboardPermissionDialog creates the dialog message via InteractionRespond(ChannelMessageWithSource), so after DeferredMessageUpdate the correct way to update that original dialog is InteractionResponseEdit (edits @original). Using FollowupMessageEdit(i.Interaction, i.Message.ID, ...) can fail because the dialog message is not a follow-up message for this interaction token, which would leave the buttons/content unchanged.

Suggested change
_, err = s.FollowupMessageEdit(i.Interaction, i.Message.ID, &edit)
_, err = s.InteractionResponseEdit(i.Interaction, &edit)

Copilot uses AI. Check for mistakes.
Content: &content,
Components: &emptyComponents,
}
_, err = s.FollowupMessageEdit(i.Interaction, i.Message.ID, &edit)
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same issue as the leaderboard permission dialog: after replying with DeferredMessageUpdate, updating the original permission dialog message should use InteractionResponseEdit (edit @original). FollowupMessageEdit(i.Interaction, i.Message.ID, ...) targets follow-up messages and may fail for this dialog message, leaving the components in place.

Suggested change
_, err = s.FollowupMessageEdit(i.Interaction, i.Message.ID, &edit)
_, err = s.InteractionResponseEdit(i.Interaction, &edit)

Copilot uses AI. Check for mistakes.

case "close":
respondAndClose("Dialog closed.")
respondAndClose("I understand")
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The updated close response text ("I understand") is ambiguous: it doesn't indicate that the dialog was closed or what the user should do next. Consider keeping a clearer message like “Dialog closed.” (and, for coop-status, the prior guidance about enabling later) so users understand the outcome.

Suggested change
respondAndClose("I understand")
respondAndClose("Dialog closed.")

Copilot uses AI. Check for mistakes.

case "close":
respondAndClose("Dialog closed. You can enable this permission later when you're ready.")
respondAndClose("I understand")
Copy link

Copilot AI Apr 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The close response text was changed to "I understand", which removes the previous guidance (“Dialog closed… enable later…”). This makes it unclear that the dialog was dismissed and that the user can re-enable permission later; consider restoring a more descriptive close message.

Suggested change
respondAndClose("I understand")
respondAndClose("Dialog closed. You can enable coop status permission later if needed.")

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants